refactor(qt): UI refresh (1/n, runtime show/hide of "Governance" and "Masternode" tabs, split out proposal model, additional bounds checking, better QTextEdit styling) #7112
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds a Qt governance proposal model and UI components (Proposal, ProposalModel, ProposalList) to parse and display CGovernanceObject data. Introduces DescriptionDialog replacing TransactionDescDialog. OptionsModel gains showMasternodes/showGovernance flags and emits showMasternodesChanged/showGovernanceChanged/showCoinJoinChanged. BitcoinGUI and WalletView always create masternode and governance pages and add updateGovernanceVisibility/updateMasternodesVisibility to toggle toolbar actions and buttons. GUI font utilities add getScaledFont/getFontBold and per-QTextEdit HTML registration. Various signal connections updated from coinJoinEnabledChanged to showCoinJoinChanged. Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/qt/guiutil_font.h (1)
1-178: Pipeline failure: clang-format differences detected.The CI reports formatting issues in this file. Run
clang-formator the project's diff tool to fix formatting before merging.
🤖 Fix all issues with AI agents
In `@src/qt/governancelist.cpp`:
- Around line 186-188: The context menu action currently captures a raw
Proposal* (proposal) which can dangle if reconcile() modifies m_data before the
user clicks; change the lambda to capture a stable identifier (e.g.,
proposal->getHash() or id) instead of the pointer and, inside the action, look
up the Proposal from m_data (or the container that owns proposals) and only call
openUrl() if the lookup returns a valid pointer; alternatively, ensure
proposalContextMenu is disabled/cleared during reconcile() updates to avoid the
race. Reference: proposalContextMenu, proposal_url, Proposal, reconcile(),
m_data.
In `@src/qt/guiutil_font.cpp`:
- Around line 619-625: getScaledFont currently calls
g_font_registry.GetScaledFontSize(...) before calling getFont, causing the size
to be scaled twice; change getScaledFont to pass the unscaled size (baseSize *
multiplier) into getFont and let getFont (which calls
g_font_registry.GetScaledFontSize internally) perform the actual scaling. Locate
getScaledFont and replace the g_font_registry.GetScaledFontSize(...) argument
with the raw baseSize * multiplier while keeping the weight selection logic the
same.
🧹 Nitpick comments (5)
src/qt/proposalmodel.h (2)
20-48: Good design for the Proposal wrapper class.The class cleanly encapsulates governance object data with cached QString fields. A few observations:
The
clientModelraw pointer is acceptable here sinceProposaldoesn't own it, but methods likeisActive(),GetAbsoluteYesCount(), andvotingStatus()dereference it without null checks. If aProposalis ever used afterclientModelbecomes invalid, this could cause a crash.Consider adding a brief comment documenting the lifetime requirement (e.g.,
clientModelmust outlive theProposal).
89-91: Minor: Redundant empty check inisValidRow.The
!m_data.empty()check is redundant sincerow >= 0 && row < rowCount()already handles the empty case (whenm_datais empty,rowCount()returns 0, so anyrow >= 0will fail therow < rowCount()check).Simplified version
- bool isValidRow(int row) const { return !m_data.empty() && row >= 0 && row < rowCount(); } + bool isValidRow(int row) const { return row >= 0 && row < rowCount(); }src/qt/proposalmodel.cpp (3)
28-32: Early return leaves Proposal in partially initialized state.If
prop_data.read()fails, the constructor returns early withm_hashset but all other fields (m_title,m_startDate,m_endDate,m_paymentAmount,m_url) at their default values. This could result in proposals being displayed with a hash but empty title and dates.Consider either:
- Throwing an exception or using a factory that returns
std::optional<Proposal>- Adding an
isValid()method to check if parsing succeeded- Documenting this behavior is intentional for malformed governance objects
95-103: Unused parameter warnings.The
indexparameter is unused inrowCount()andcolumnCount(), which is expected for flat (non-hierarchical) models. Consider usingQ_UNUSEDto suppress compiler warnings.Suppress unused parameter warnings
int ProposalModel::rowCount(const QModelIndex& index) const { + Q_UNUSED(index) return static_cast<int>(m_data.size()); } int ProposalModel::columnCount(const QModelIndex& index) const { + Q_UNUSED(index) return Column::_COUNT; }
136-164: Unreachable code:breakstatements after return.Lines 138 and 161 contain
breakstatements that are unreachable because the inner switch cases either return or fall through todefaultwhich returns. The outer switch at line 163 also has unreachable code paths.Remove unreachable breaks
default: return {}; - }; - break; + } } case Qt::EditRole: { // Edit role is used for sorting, so return the raw values where possible switch (index.column()) { ... default: return {}; - }; - break; + } } - }; + } return {};
| proposalContextMenu->clear(); | ||
| proposalContextMenu->addAction(proposal_url, proposal, &Proposal::openUrl); | ||
| proposalContextMenu->addAction(proposal_url, [proposal]() { proposal->openUrl(); }); | ||
|
|
There was a problem hiding this comment.
Potential dangling pointer in context menu lambda.
The lambda [proposal]() { proposal->openUrl(); } captures a raw pointer to a Proposal owned by m_data. If reconcile() is called between when the context menu is displayed and when the user clicks the action (removing this proposal), the captured pointer would dangle.
This is a low-probability race condition since:
- The update timer interval is typically several seconds
- The proposal would need to be removed in that exact window
Consider capturing by hash and looking up the proposal when the action is triggered, or disabling the menu during updates.
Safer alternative using hash lookup
// right click menu with option to open proposal url
- QString proposal_url = proposal->url();
+ const QString proposal_url = proposal->url();
+ const QString proposal_hash = proposal->hash();
proposal_url.replace(QChar('&'), QString("&&"));
proposalContextMenu->clear();
- proposalContextMenu->addAction(proposal_url, [proposal]() { proposal->openUrl(); });
+ proposalContextMenu->addAction(proposal_url, [this, proposal_hash]() {
+ // Re-lookup proposal to avoid dangling pointer if reconcile() ran
+ for (int row = 0; row < proposalModel->rowCount(); ++row) {
+ if (auto* p = proposalModel->getProposalAt(proposalModel->index(row, 0));
+ p && p->hash() == proposal_hash) {
+ p->openUrl();
+ return;
+ }
+ }
+ });🤖 Prompt for AI Agents
In `@src/qt/governancelist.cpp` around lines 186 - 188, The context menu action
currently captures a raw Proposal* (proposal) which can dangle if reconcile()
modifies m_data before the user clicks; change the lambda to capture a stable
identifier (e.g., proposal->getHash() or id) instead of the pointer and, inside
the action, look up the Proposal from m_data (or the container that owns
proposals) and only call openUrl() if the lookup returns a valid pointer;
alternatively, ensure proposalContextMenu is disabled/cleared during reconcile()
updates to avoid the race. Reference: proposalContextMenu, proposal_url,
Proposal, reconcile(), m_data.
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
- CoinJoin live updates are broken, see below
- text format in tx description dialog is broken, there is no bold text anymore
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/qt/forms/descriptiondialog.ui`:
- Around line 13-15: Remove the notr="true" attribute on the window title string
in descriptiondialog.ui so the title becomes translatable; locate the <property
name="windowTitle"> element (in descriptiondialog.ui) and change the <string
notr="true">Details</string> entry to a plain <string>Details</string> (matching
other dialogs' translatable titles) so translation tooling will pick it up.
In `@src/qt/proposalmodel.cpp`:
- Around line 1-4: The file proposalmodel.cpp is failing the clang-format check;
run the project's clang-format configuration on proposalmodel.cpp (e.g.,
clang-format -i proposalmodel.cpp or via your IDE/CI formatter) to apply the
canonical formatting, verify changes cover all edits in the ProposalModel
implementation, then stage and commit the formatted file so the CI clang-format
check passes.
- Around line 82-85: The Proposal::openUrl method currently passes m_url
directly to QDesktopServices::openUrl, which can open unsafe schemes; change it
to parse the input with QUrl::fromUserInput(m_url), validate the result with
QUrl::isValid(), then check QUrl::scheme() against an allowlist ("http" and
"https") before calling QDesktopServices::openUrl; if validation fails, do not
open the URL (optionally log or ignore) to prevent file:// or custom-protocol
exploitation.
In `@src/qt/proposalmodel.h`:
- Around line 85-86: setDisplayUnit currently updates m_display_unit but does
not notify views, so the Amount column stays in the old unit; modify
ProposalModel::setDisplayUnit to emit dataChanged for all rows in the
PAYMENT_AMOUNT column after setting m_display_unit (use QModelIndex created from
index(0, PAYMENT_AMOUNT) to index(rowCount()-1, PAYMENT_AMOUNT) or equivalent)
so the view refreshes the displayed amounts; keep the function signature
setDisplayUnit(const BitcoinUnit& display_unit) and ensure the emission is done
on the model instance (this->dataChanged(...)).
🧹 Nitpick comments (2)
src/qt/descriptiondialog.cpp (1)
6-11: Fix clang-format differences flagged by CI.The pipeline reports formatting differences in this file. Run
clang-format-diff.pyorclang-formatto resolve the issue before merging.Additionally, consider reordering the includes to place the own header (
descriptiondialog.h) before the generated UI header for consistency with typical conventions:Suggested include order
-#include <qt/forms/ui_descriptiondialog.h> - `#include` <qt/descriptiondialog.h> + +#include <qt/forms/ui_descriptiondialog.h> `#include` <qt/guiutil_font.h>src/qt/guiutil_font.cpp (1)
207-259: Consider documenting the heading scale factors.The
setFontBodyHTMLfunction applies font styling to QTextEdit widgets with HTML content. The heading scale factors (0.10, 0.04, 0.02 for h1-h3) are hardcoded magic numbers. Consider extracting these as named constants for clarity and maintainability.♻️ Suggested improvement
+namespace { +// Scale additions for heading levels relative to base font size +constexpr double HEADING_H1_SCALE = 0.10; +constexpr double HEADING_H2_SCALE = 0.04; +constexpr double HEADING_H3_SCALE = 0.02; +} // namespace + void setFontBodyHTML(QTextEdit* widget, const QString& src, double base_size) { // ... existing code ... switch (heading_level) { case 1: - scale_add = 0.10; + scale_add = HEADING_H1_SCALE; break; case 2: - scale_add = 0.04; + scale_add = HEADING_H2_SCALE; break; case 3: - scale_add = 0.02; + scale_add = HEADING_H3_SCALE; break;
| <property name="windowTitle"> | ||
| <string notr="true">Transaction details</string> | ||
| <string notr="true">Details</string> | ||
| </property> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and view its full content
cat -n src/qt/forms/descriptiondialog.uiRepository: dashpay/dash
Length of output: 2267
🏁 Script executed:
# Search for other .ui files to understand localization patterns
fd -e ui src/qt/forms/ -x wc -l {} +Repository: dashpay/dash
Length of output: 413
🏁 Script executed:
# Check for other uses of notr in .ui files
rg 'notr="true"' --type xml src/qt/forms/Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check if there are other windowTitle definitions to understand the pattern
rg 'windowTitle' --type xml src/qt/forms/ -A 1 -B 1Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Search for .ui files and check notr patterns
fd --full-path 'src/qt/forms' -name '*.ui' | head -5Repository: dashpay/dash
Length of output: 286
🏁 Script executed:
# Check notr usage in .ui files
rg 'notr="true"' src/qt/forms/ --type xml | head -20Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check windowTitle patterns in .ui files (without and with notr)
rg '<string' src/qt/forms/descriptiondialog.ui -B 1Repository: dashpay/dash
Length of output: 221
🏁 Script executed:
# Search for windowTitle patterns in other .ui files to compare
rg 'windowTitle' src/qt/forms/ --type xml -A 1 | head -30Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Find all .ui files in the qt/forms directory
find src/qt/forms -name '*.ui' | sortRepository: dashpay/dash
Length of output: 868
🏁 Script executed:
# Count occurrences of notr in all .ui files
rg 'notr' src/qt/formsRepository: dashpay/dash
Length of output: 5948
🏁 Script executed:
# Check all windowTitle definitions across .ui files
rg 'windowTitle' src/qt/forms -A 1 | head -50Repository: dashpay/dash
Length of output: 2335
Remove notr attribute from the window title to make it translatable.
All other dialog window titles in the codebase are translatable. This is the only window title marked notr="true", and the same dialog has translatable tooltip content, suggesting this was unintentional.
Suggested fix
- <string notr="true">Details</string>
+ <string>Details</string>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <property name="windowTitle"> | |
| <string notr="true">Transaction details</string> | |
| <string notr="true">Details</string> | |
| </property> | |
| <property name="windowTitle"> | |
| <string>Details</string> | |
| </property> |
🤖 Prompt for AI Agents
In `@src/qt/forms/descriptiondialog.ui` around lines 13 - 15, Remove the
notr="true" attribute on the window title string in descriptiondialog.ui so the
title becomes translatable; locate the <property name="windowTitle"> element (in
descriptiondialog.ui) and change the <string notr="true">Details</string> entry
to a plain <string>Details</string> (matching other dialogs' translatable
titles) so translation tooling will pick it up.
| // Copyright (c) 2021-2026 The Dash Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
There was a problem hiding this comment.
Clang-format check failing in this file.
CI reports clang-format differences for proposalmodel.cpp. Please run the formatter on this file to unblock the pipeline.
🧰 Tools
🪛 GitHub Actions: Clang Diff Format Check
[error] 1-1: Clang format differences detected in proposalmodel.cpp. Run clang-format-diff.py or clang-format to fix formatting.
🤖 Prompt for AI Agents
In `@src/qt/proposalmodel.cpp` around lines 1 - 4, The file proposalmodel.cpp is
failing the clang-format check; run the project's clang-format configuration on
proposalmodel.cpp (e.g., clang-format -i proposalmodel.cpp or via your IDE/CI
formatter) to apply the canonical formatting, verify changes cover all edits in
the ProposalModel implementation, then stage and commit the formatted file so
the CI clang-format check passes.
| void Proposal::openUrl() const | ||
| { | ||
| QDesktopServices::openUrl(QUrl(m_url)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the proposalmodel.cpp file
find . -name "proposalmodel.cpp" -type fRepository: dashpay/dash
Length of output: 82
🏁 Script executed:
# Check the file size and read the relevant section
wc -l src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 84
🏁 Script executed:
# Read the code around lines 82-85 and surrounding context
sed -n '70,100p' src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 1011
🏁 Script executed:
# Search for where m_url is assigned/initialized in this file
rg -n "m_url\s*=" src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 118
🏁 Script executed:
# Check the header file for the Proposal class definition
find . -name "proposalmodel.h" -type fRepository: dashpay/dash
Length of output: 80
🏁 Script executed:
# Look for URL validation patterns elsewhere in the Qt code
rg -n "QUrl\|openUrl\|scheme" src/qt/ -A 2 -B 2 | head -80Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Read the header file to understand the Proposal class
cat -n src/qt/proposalmodel.h | head -150Repository: dashpay/dash
Length of output: 3341
🏁 Script executed:
# Check the context around line 47 to understand where m_url comes from
sed -n '40,60p' src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 649
🏁 Script executed:
# Look for Qt version or CMake configuration
find . -name "CMakeLists.txt" -o -name ".cmake" | head -5Repository: dashpay/dash
Length of output: 219
🏁 Script executed:
# Search for other uses of QDesktopServices::openUrl in the codebase
rg -n "QDesktopServices::openUrl\|openUrl" src/qt/ -B 3 -A 3Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check if there are any existing URL validation patterns in the Qt code
rg -n "QUrl\|scheme\|isValid" src/qt/ -A 1 | head -100Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check Qt configuration in CMakeLists.txt
grep -n "Qt\|Qt5\|Qt6" CMakeLists.txt | head -20Repository: dashpay/dash
Length of output: 119
🏁 Script executed:
# Search for find_package Qt in CMakeLists.txt
grep -n "find_package" CMakeLists.txt | grep -i qtRepository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Look for existing URL validation or scheme checking in the entire codebase
rg -n "scheme\(\)" src/qt/ -B 2 -A 2Repository: dashpay/dash
Length of output: 306
🏁 Script executed:
# Check if there's any existing input validation for on-chain data
rg -n "isValid\|validation" src/qt/ -B 1 -A 1 | head -60Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Search for documentation or comments about proposal URLs
rg -n "url\|URL\|Url" src/qt/proposalmodel.cpp -B 2 -A 2Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check what QDesktopServices can do with different schemes
rg -n "QDesktopServices\|openUrl" . --type cpp -B 3 -A 3 | head -40Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Look for Qt version specification in CMakeLists.txt
grep -n "Qt5\|Qt6\|CMAKE_AUTOMOC\|QtWidgets" CMakeLists.txt | head -30Repository: dashpay/dash
Length of output: 119
🏁 Script executed:
# Check if QUrl::fromUserInput is used anywhere in the codebase
rg -n "fromUserInput" src/qt/ -B 2 -A 2Repository: dashpay/dash
Length of output: 470
🏁 Script executed:
# Look at the full context of the URL validation pattern in guiutil.cpp
sed -n '310,330p' src/qt/guiutil.cppRepository: dashpay/dash
Length of output: 780
🏁 Script executed:
# Search for all URL/URI handling patterns in the Qt code
rg -n "QUrl\(" src/qt/ -B 1 -A 1 | head -80Repository: dashpay/dash
Length of output: 592
🏁 Script executed:
# Check if there are any file:// or protocol-related security concerns mentioned elsewhere
rg -n "file://\|scheme\|protocol\|openUrl" . --type cpp --type h | grep -i "file\|scheme\|security\|safe" | head -20Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check if there's a specific reason proposal URLs might need to support other schemes
rg -n "proposal.*url\|openUrl.*proposal" src/ --type cpp --type h -i -B 2 -A 2Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Verify Qt5 is the target version by checking the main CMakeLists.txt more thoroughly
head -100 CMakeLists.txt | grep -i "qt\|version"Repository: dashpay/dash
Length of output: 514
🏁 Script executed:
# Check if there are any recent changes or discussions about URL handling in proposals
git log --oneline --all -- src/qt/proposalmodel.cpp | head -10Repository: dashpay/dash
Length of output: 138
🏁 Script executed:
# Look at the transactionview.cpp more closely to understand the pattern of using QUrl::fromUserInput
sed -n '580,600p' src/qt/transactionview.cppRepository: dashpay/dash
Length of output: 942
🏁 Script executed:
# Check what happens when you use QUrl constructor vs fromUserInput with invalid URLs
python3 << 'EOF'
# Document the difference between QUrl(string) and QUrl::fromUserInput(string)
# QUrl(string) - strict parsing, treats the string as already formatted
# fromUserInput - attempts to be more forgiving, treats input as user-typed
# For security with on-chain data, the suggested approach is sound
print("QUrl(m_url) - strict mode, accepts arbitrary schemes")
print("QUrl::fromUserInput(m_url) - forgiving mode, better for untrusted input")
print("\nThe suggestion to validate scheme is appropriate for on-chain data")
print("Precedent exists in transactionview.cpp for this pattern")
EOFRepository: dashpay/dash
Length of output: 142
🏁 Script executed:
# Check if there are any comments or documentation about what schemes proposal URLs should support
sed -n '1,100p' src/qt/proposalmodel.cpp | grep -i "url\|scheme\|protocol\|comment" -B 1 -A 1Repository: dashpay/dash
Length of output: 337
🏁 Script executed:
# Verify Qt5 is explicitly required somewhere
rg -n "find_package.*Qt\|set.*Qt\|PROJECT" CMakeLists.txt | head -20Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check documentation on QUrl::fromUserInput availability in Qt5
rg -n "Qt5\|QtWidgets" CMakeLists.txt | head -10Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Check if there's any existing validation of proposal URLs anywhere
rg -n "m_url\|openUrl" src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Verify QUrl::fromUserInput is available and used in Qt5 codebase
rg -n "fromUserInput" src/qt/ -cRepository: dashpay/dash
Length of output: 84
🏁 Script executed:
# Check if there are any other security concerns with proposal data handling
rg -n "proposal.*url\|url.*proposal" src/ --type cpp -i -B 1 -A 1Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Double check the exact context of how openUrl is called - is there any wrapper?
sed -n '75,95p' src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 557
🏁 Script executed:
# Verify there's no validation happening at the call site
rg -n "openUrl\|Proposal.*openUrl" src/qt/ -B 3 -A 3Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Final check - see if there are any command execution or security discussions in commit history
git log --all --oneline --grep="url\|URL\|scheme\|security" | head -20Repository: dashpay/dash
Length of output: 2235
Sanitize proposal URLs before opening them.
Proposal URLs are on-chain governance data and QDesktopServices::openUrl() accepts arbitrary schemes, creating a risk for file:// and custom protocol exploitation. Use QUrl::fromUserInput() with scheme validation to allowlist only safe schemes (http/https).
Suggested hardening
void Proposal::openUrl() const
{
- QDesktopServices::openUrl(QUrl(m_url));
+ const QUrl url = QUrl::fromUserInput(m_url);
+ if (!url.isValid() || (url.scheme() != "http" && url.scheme() != "https")) {
+ return;
+ }
+ QDesktopServices::openUrl(url);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void Proposal::openUrl() const | |
| { | |
| QDesktopServices::openUrl(QUrl(m_url)); | |
| } | |
| void Proposal::openUrl() const | |
| { | |
| const QUrl url = QUrl::fromUserInput(m_url); | |
| if (!url.isValid() || (url.scheme() != "http" && url.scheme() != "https")) { | |
| return; | |
| } | |
| QDesktopServices::openUrl(url); | |
| } |
🤖 Prompt for AI Agents
In `@src/qt/proposalmodel.cpp` around lines 82 - 85, The Proposal::openUrl method
currently passes m_url directly to QDesktopServices::openUrl, which can open
unsafe schemes; change it to parse the input with QUrl::fromUserInput(m_url),
validate the result with QUrl::isValid(), then check QUrl::scheme() against an
allowlist ("http" and "https") before calling QDesktopServices::openUrl; if
validation fails, do not open the URL (optionally log or ignore) to prevent
file:// or custom-protocol exploitation.
| void setDisplayUnit(const BitcoinUnit& display_unit) { m_display_unit = display_unit; } | ||
| void setVotingParams(int nAbsVoteReq); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the proposalmodel.h file
fd -t f "proposalmodel.h" src/qt/Repository: dashpay/dash
Length of output: 78
🏁 Script executed:
# Read the proposalmodel.h file to understand the context
cat -n src/qt/proposalmodel.hRepository: dashpay/dash
Length of output: 3341
🏁 Script executed:
# Also check proposalmodel.cpp for implementation details
wc -l src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 84
🏁 Script executed:
# Check the data() method implementation in proposalmodel.cpp
sed -n '1,100p' src/qt/proposalmodel.cpp | cat -nRepository: dashpay/dash
Length of output: 3766
🏁 Script executed:
# Search for dataChanged usage in the file
rg -n "dataChanged" src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 300
🏁 Script executed:
# Check the full implementation of data() method
rg -A 50 "QVariant ProposalModel::data" src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 1799
🏁 Script executed:
# Check the setVotingParams implementation to see the pattern for emitting dataChanged
rg -A 15 "void ProposalModel::setVotingParams" src/qt/proposalmodel.cppRepository: dashpay/dash
Length of output: 578
🏁 Script executed:
# Check how other Qt models in the codebase handle display unit changes
rg -B 5 -A 10 "setDisplayUnit" src/qt/*.cpp | head -60Repository: dashpay/dash
Length of output: 3085
Emit dataChanged when the display unit changes.
setDisplayUnit updates the unit but never notifies the view, leaving the Amount column in the old unit until an unrelated refresh occurs. Emit dataChanged for the affected column (PAYMENT_AMOUNT).
♻️ Suggested fix
- void setDisplayUnit(const BitcoinUnit& display_unit) { m_display_unit = display_unit; }
+ void setDisplayUnit(const BitcoinUnit& display_unit)
+ {
+ if (m_display_unit == display_unit) return;
+ m_display_unit = display_unit;
+ if (!m_data.empty()) {
+ Q_EMIT dataChanged(createIndex(0, Column::PAYMENT_AMOUNT),
+ createIndex(rowCount() - 1, Column::PAYMENT_AMOUNT));
+ }
+ }🤖 Prompt for AI Agents
In `@src/qt/proposalmodel.h` around lines 85 - 86, setDisplayUnit currently
updates m_display_unit but does not notify views, so the Amount column stays in
the old unit; modify ProposalModel::setDisplayUnit to emit dataChanged for all
rows in the PAYMENT_AMOUNT column after setting m_display_unit (use QModelIndex
created from index(0, PAYMENT_AMOUNT) to index(rowCount()-1, PAYMENT_AMOUNT) or
equivalent) so the view refreshes the displayed amounts; keep the function
signature setDisplayUnit(const BitcoinUnit& display_unit) and ensure the
emission is done on the model instance (this->dataChanged(...)).
still not fixed for me |
|
@UdjinM6, unable to reproduce the issue locally (see below) but develop on my machine doesn't show bold font (see PR description) so it's likely some platform-specific behaviour is now inverted
|
Also `s/coinJoinEnabledChanged/showCoinJoinChanged/g` to establish a naming convention that will be used soon.
This should fix the misalignment of the `subFeeFromAmount` `QCheckBox` compared to the rest of the `QCheckBox`es Review with `git log -p -n1 --ignore-space-change --color-moved=dimmed_zebra`.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Inheriting the whole structure is overkill just to use `tr` and `hash()` is used in hot loops so we should cache the value in the ctor. Also remove dead variable.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/qt/guiutil_font.cpp`:
- Around line 569-572: The clang-format check is failing for the formatting
around the qDebug() call; run clang-format (or clang-format-diff) on
src/qt/guiutil_font.cpp to reflow the long qDebug().nospace() << qstrprintf(...)
expression (the qDebug().nospace(), qstrprintf call and __func__ usage) so it
matches the project's style rules, then stage the formatted file; ensure the
qDebug/qstrprintf line and surrounding lines are reformatted and no clang-format
diffs remain.
In `@src/qt/guiutil_font.h`:
- Line 85: The GetScaledFontSize method's formatting violates the project's
clang-format rules; run clang-format (or clang-format-diff) on
src/qt/guiutil_font.h and reformat the GetScaledFontSize declaration/definition
so it matches the project's style (e.g., break long inline expression across
lines according to the configured style) while leaving the logic using
m_font_scale and m_scale_steps unchanged; commit the reformatted file so CI
clang-format check passes.
🧹 Nitpick comments (3)
src/qt/descriptiondialog.cpp (1)
6-11: Include order: own header should precede generated UI header.The convention in this codebase is to include the class's own header before generated UI headers. Additionally, the pipeline reports clang-format differences.
♻️ Suggested include order fix
-#include <qt/forms/ui_descriptiondialog.h> - `#include` <qt/descriptiondialog.h> + +#include <qt/forms/ui_descriptiondialog.h> `#include` <qt/guiutil_font.h> - `#include` <qt/guiutil.h>src/qt/proposalmodel.cpp (2)
57-61: Consider adding a null check forclientModel.While the constructor stores the pointer, if
clientModelbecomes null later (unlikely but possible), this would crash. Given that this is a UI component tied to model lifecycle, the current approach is acceptable, but a defensive check could improve robustness.
91-94: Consider marking unused parameters with[[maybe_unused]].The
indexparameter inrowCount()andcolumnCount()is unused, which is correct for flat (non-hierarchical) models. Adding the attribute would silence compiler warnings and document intent.🔧 Suggested improvement
-int ProposalModel::rowCount(const QModelIndex& index) const +int ProposalModel::rowCount([[maybe_unused]] const QModelIndex& index) const { return static_cast<int>(m_data.size()); } -int ProposalModel::columnCount(const QModelIndex& index) const +int ProposalModel::columnCount([[maybe_unused]] const QModelIndex& index) const { return Column::_COUNT; }Also applies to: 96-99
| qDebug().nospace() << qstrprintf("%s - widget counts: updated/updatable/total(%d/%d/%d), removed items: " | ||
| "mapWidgetDefaultFontSizes/mapFontUpdates(%d/%d)", | ||
| "mapWidgetDefaultFontSizes/mapFontUpdates/mapTextEditStyleUpdates(%d/%d/%d)", | ||
| __func__, nUpdated, nUpdatable, qApp->allWidgets().size(), nRemovedDefaultFonts, | ||
| nRemovedFontUpdates); | ||
| nRemovedFontUpdates, nRemovedTextEditUpdates); |
There was a problem hiding this comment.
Clang-format check is failing for this file.
CI reports clang-format diffs here; please run clang-format/clang-format-diff on this file before merge.
🤖 Prompt for AI Agents
In `@src/qt/guiutil_font.cpp` around lines 569 - 572, The clang-format check is
failing for the formatting around the qDebug() call; run clang-format (or
clang-format-diff) on src/qt/guiutil_font.cpp to reflow the long
qDebug().nospace() << qstrprintf(...) expression (the qDebug().nospace(),
qstrprintf call and __func__ usage) so it matches the project's style rules,
then stage the formatted file; ensure the qDebug/qstrprintf line and surrounding
lines are reformatted and no clang-format diffs remain.
|
|
||
| double GetScaleSteps() const { return m_scale_steps; } | ||
| double GetScaledFontSize(int size) const { return std::round(size * (1 + (m_font_scale * m_scale_steps)) * 4) / 4.0; } | ||
| double GetScaledFontSize(double size) const { return std::round(size * (1 + (m_font_scale * m_scale_steps)) * 4) / 4.0; } |
There was a problem hiding this comment.
Clang-format check is failing for this header.
CI reports clang-format diffs here; please run clang-format/clang-format-diff on this file before merge.
🤖 Prompt for AI Agents
In `@src/qt/guiutil_font.h` at line 85, The GetScaledFontSize method's formatting
violates the project's clang-format rules; run clang-format (or
clang-format-diff) on src/qt/guiutil_font.h and reformat the GetScaledFontSize
declaration/definition so it matches the project's style (e.g., break long
inline expression across lines according to the configured style) while leaving
the logic using m_font_scale and m_scale_steps unchanged; commit the reformatted
file so CI clang-format check passes.
|
Interesting. When I open tx detail dialog I see no bold text there. But if I go into Settings -> Appearance and move the bold text slider to the left and then back to the right then it starts to look just like it would on develop. |
|
Actually.. It's not just tx details dialog, bold text settings are broken in general it seems. Pls see 52083af. This issue exists on develop so I could submit it as a separate PR if you don't want it here (would conflict with this PR though). |
2 participants
Additional Information
develop(cc50446)Breaking Changes
None expected.
Checklist